Xy screen variables - #48
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for screen centroid readback PVs (X, Y) so screen devices expose centroid readbacks consistently (similar to BPMs), and updates the test utilities/tests to validate the expanded screen PV coverage.
Changes:
- Added Cheetah action variables for screen centroid readbacks (
ScreenXVariable,ScreenYVariable) and wired them into the screen PV mapping. - Updated BMAD screen variable generation to include
:X/:YPVs for screen elements. - Refactored test utilities to centralize default PV suffix lists and updated CU HXR tests to use the consolidated helpers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
virtual_accelerator/cheetah/actions.py |
Adds read-only centroid variables for screens (X/Y) with unit conversion. |
virtual_accelerator/cheetah/variables.py |
Updates the screen PV suffix → variable-class mapping to route X/Y to the new centroid variables. |
virtual_accelerator/bmad/variables.py |
Extends BMAD screen variables to expose :X/:Y readbacks. |
virtual_accelerator/tests/_bmad_model_test_utils.py |
Centralizes default PV attribute lists and updates screen/BPM/magnet helper defaults accordingly. |
virtual_accelerator/tests/test_cu_hxr.py |
Updates CU HXR tests to use the renamed/updated screen PV coverage helpers and adjusts expected supported-variable count. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces support for screen centroid readback variables (
XandY) in the virtual accelerator, aligning screen PVs with BPMs and updating both implementation and tests to ensure coverage and consistency. The main changes include new variable classes for screen centroids, updates to variable mappings, and consolidation of test PV attribute lists.Screen centroid variable support:
ScreenCentroidVariable,ScreenXVariable, andScreenYVariableclasses to provide read-only access to the beam centroid positions for screens, with automatic conversion to millimeters. (virtual_accelerator/cheetah/actions.py)get_screen_variablesto includeBPMXVariableandBPMYVariablefor screen elements, enabling exposure ofXandYPVs. (virtual_accelerator/bmad/variables.py)Variable mapping updates:
"X"and"Y"PVs now use the newScreenXVariableandScreenYVariableclasses, replacing the old array size mappings. (virtual_accelerator/cheetah/variables.py)Test improvements and refactoring:
virtual_accelerator/tests/_bmad_model_test_utils.py) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]test_cu_hxr.pyto validate the new screen PVs and ensure that screen PV coverage matches expectations. (virtual_accelerator/tests/test_cu_hxr.py) [1] [2] [3] [4]Dependency update:
Screenclass incheetah/actions.pyto support type checking for the new screen centroid variables. (virtual_accelerator/cheetah/actions.py)